Rewrite sliding window bounds let at the correct loop level#9150
Conversation
Sliding window populates a replacements map keyed by .min/.max let names and rewrites the first matching LetStmt encountered on the way out from the producer. That happens to hit the right let today because bounds inference places the lets driving production at the producer's loop nest level, but other lets with the same names can appear at deeper loop nest levels on the consume side. Gate replacement on enclosing_loops.size() matching the depth recorded when the producer was visited, so only lets at the producer's loop level are rewritten. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
I'm not sure how the depth eliminates ambiguity. Does it merely make it less likely? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9150 +/- ##
=======================================
Coverage ? 69.41%
=======================================
Files ? 254
Lines ? 78221
Branches ? 18714
=======================================
Hits ? 54301
Misses ? 18470
Partials ? 5450 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
There should be a single set of .min/.max lets per loop. But I guess you're saying there could be sibling loops at the depth of the produce node, multiple of which contain .min/.max lets, but only one of which actually contains the produce node? Perhaps I should change it to check the loop name instead of the depth. |
|
|
abadams
force-pushed
the
abadams/sliding_window_bounds_rewrite_level
branch
from
fe571ed to
9c2fe4a
Compare
alexreinking
left a comment
alexreinking
left a comment
There was a problem hiding this comment.
This looks like the correct fix now. Please update the PR / commit description.
…w_bounds_rewrite_level
2 participants
Sliding window populates a replacements map keyed by .min/.max let names and rewrites the first matching LetStmt encountered on the way out from the producer. That happens to hit the right let today because we don't inject bounds for that Func anywhere else that we might hit first (e.g. inside a loop on the consumer side, which is visited after the producer). That's a shaky assumption though. It caused a bug in another branch where I added more .min/.max declaration sites for other purposes. This PR explicitly only rewrites the ones immediately inside the Produce node's loop. No test changes because there's no bug to trigger on main. This just makes the code more robust to future changes.